Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

monitor: refactor MonitorHandle to store dyn object #3927

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kchibisov
Copy link
Member

This also alters VideoMode to be a regular object and not reference the MonitorHandle, since it's a static data.

Given that VideoMode set may change during runtime keeping the reference as a some sort of validity may not be idea and propagating errors when changing video mode could be more reliable.

--

I'm not sure that I like how the monitor stuff looks, but I don't have a better idea. Ideally we want to have some sort of MonitorId which is more reliable in representing and split the MonitorHandle into Id and a way to get the data for the monitor, but I think I'd rather leave it for the future.

The native_id thing I've moved is already present on all the platforms, so I'd really see why not and usually platforms have some way to address the monitors and backends may build such addressing themselves.

It could make sense to change native_id() -> u64 to MonitorId(u64).

@kchibisov kchibisov marked this pull request as ready for review November 14, 2024 16:15
@kchibisov kchibisov requested review from notgull and removed request for notgull November 14, 2024 16:15
@kchibisov kchibisov force-pushed the kchibisov/dyn-monitor-handle branch 3 times, most recently from f2b84f4 to 84c9ae6 Compare November 14, 2024 17:08
@kchibisov
Copy link
Member Author

kchibisov commented Nov 14, 2024

I've removed docsrs build for platform for now, since types are simply not present in all backends, so build fails), and platform module will be soon gone anyway.

@kchibisov kchibisov force-pushed the kchibisov/dyn-monitor-handle branch 4 times, most recently from 8565120 to 9486937 Compare November 14, 2024 17:37
This also alters `VideoMode` to be a regular object and not reference
the `MonitorHandle`, since it's a static data.

Given that `VideoMode` set may change during runtime keeping the
reference as a some sort of validity may not be idea and propagating
errors when changing video mode could be more reliable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant